feat: environment api #16129
feat: environment api #16129
Conversation
|
|
patak-dev
added
feat: hmr
feat: dev
packages/vite/src/node/plugin.ts
Outdated
| options?: { ssr?: boolean; environment?: string }, | ||
| options?: { | ||
| ssr?: boolean | ||
| environment?: ModuleExecutionEnvironment | BuildEnvironment |
There was a problem hiding this comment.
Can we guarantee it exists somehow? (non undefined)
There was a problem hiding this comment.
The problem is that during scan, we didn't pass the moduleGraph. See here
vite/packages/vite/src/node/optimizer/scan.ts
Line 212 in d2839bc
So I didn't pass the environment either. I thought that it may make the scan slower if we need to update info in a module graph that we are going to throw away. But I think it is wrong that we don't pass an environment, more once we will have environment specific config. I'll check what needs to be done here. I think we only need the plugins for resolveId that shouldn't mess with the moduleGraph, so maybe we can create a new temporal browser environment with a new module graph and pass that. I'll check this out, it would be a lot better if there is always an environment defined.
There was a problem hiding this comment.
Let's see if there isn't some internalish use cases of these hooks in the ecosystem, but for now, core works with options and environment being required for hooks. See e30b858
I fixed some issues were I missed passing the environment, so even if we revert, it was worth it. We currently have a design issue because an environment needs a resolveId function when it is created, that should be implemented as container.resolveId(id, undefined, { environment }). I'll rework this next so we can simplify the code. Edit: done in 03d3889
There was a problem hiding this comment.
It seems we'll need to relax the types. See failing test for Vitest. We could leave them as is for a bit more if they help us work with this draft though.
There was a problem hiding this comment.
I already moved back to optional options and environment. If not, we were forced to have a new GlobalEnvironment that would be used when we don't have a server available (in the plugin containers created during config resolution for example).
Still, this was quite helpful to identify issues. And things looks better after fcebb7d
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
📝 Ran ecosystem CI on
✅ analogjs, histoire, ladle, laravel, marko, previewjs, qwik, rakkas, sveltekit, unocss, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest |
Description
Important
The Environment API work has been consolidated into #16471
Work in progress implementation of the design outlined at:
Some of the changes that need to happen:
server.moduleGraphEnvironment { id, type, config }instead of stringmoduleGraph.safeModulePathstoserver.safeModulePathsssrFixStacktrace, maybe it isn't needed anymorehotUpdatehooktransformRequestet al to the environmentserver.hotto the environmentssrEnvironmentTo be implemented in future PRs:
ssrErrorandssrModuleonce ssrLoadModule is using the new APIsvite buildrunvsimportapplyoptionWe can keep improving and discussing the spec in #16089 and later on merge it as the docs PR for this feature. Sending this PR so we have a branch were we can collaborate in the implementation.
The starting point for this PR is a squash of:
#16003 implements the separation of the module graphs for the browser and server environments. Internal plugins in Vite have been refactored to use the independent module graphs. A backward compatibility later has been implemented so
server.moduleGraphworks for downstream projects in the ecosystem. Only Nuxt and Astro are failing right now in vite-ecosystem-ci for it. We can keep working on fixing backward compatibility in that PR and then merge the fixes here.So far, I moved the the each
EnvironmentModuleGraphinside ofModuleExecutionEnvironment.Some decisions that diverge from the current spec and we still need to discuss (there or here):
server.environments: Map<string, ModuleExecutionEnvironment>for now. The code is usingserver.environments.get(environmentId)ModuleExecutionEnvironmenthas anidthat is unique for the server, and atype. The idea here is to allow for several dev environments of the same type (like workerd) to be used at the same time.nodeEnvironmentbeing callednode. If at one point the Vite dev server may be run in a non-node environment, the name may not be the best. Imagine if the landscape evolves into us wanting to use only a standard subset that works in more environments for Vite core. I don't know a better name though,serverEnvironment(as in "the same environment as the server") doesn't work that well either.What is the purpose of this pull request?